Update Python integration #15

taion · 2017-12-27T00:06:23Z

Not quite ready to go yet. Need to fix tests.

taion · 2017-12-27T00:39:43Z

Should be good to go now. Might need to iterate a bit on the pyenv caching – it's better to cache the artefacts in .pyenv than the build cache.

taion · 2017-12-27T01:01:25Z

tests/python_with/__snapshots__/jsfmt.spec.js.snap

-with open("tmp/index.html"):
-    print("great")
-
-with A() as X:


what the what?

taion · 2017-12-27T01:37:03Z

Got a fix coming in a bit. This is all prep for #8.

taion · 2017-12-27T22:16:41Z

src/printer/index.js

-      ]);
-    }
-
-    case "withitem": {


no such node type

taion · 2017-12-27T22:17:00Z

src/printer/index.js

+
+  const n = path.getValue();
+  if (n.body.length === 1 && n.body[0].ast_type === "With") {
+    path.each(p => {


is this idiomatic? i couldn't find a better way but i might not have looked hard enough

the idea is that body is an array, but in this case it's of length 1, so we want to call something on it

Maybe:

return concat( path.map( p => printPython2With(p, items, print), "body" ) )

Oops. Now I feel embarrassed for not thinking of that. Thanks!

taion · 2017-12-27T22:23:42Z

src/printer/index.js

@@ -631,6 +631,10 @@ function genericPrint(path, options, print) {
        : printPython2With(path, [], print); // Python 2
    }

+    case "withitem": {


taion · 2017-12-28T10:02:07Z

.travis.yml

+env:
+  global:
+    - PYTHON_VERSIONS="2.7.11 3.6.3"
+  matrix:


this runs a separate build for each version of python under test, rather than a single build that tests both pythons

taion · 2017-12-28T10:03:27Z

python/prettier/__init__.py

+
+base_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+
+sys.path.insert(0, os.path.join(base_dir, 'vendor'))


I've seen this pattern used elsewhere for CLI-y things where messing with the global Python path isn't a problem. I like that this hides the vendoring details from the caller.

It's also good to split out vendored things from things that are patched from upstream.

taion · 2017-12-28T10:03:58Z

src/parser.js

-  if (error) {
-    throw new Error(error);
+function parse(text) {
+  const executionResult = spawnSync("python", ["-m", "prettier.parser"], {


you get a bit more flexibility when you run something as a module... you can do things like relative imports, which are sorta handy

taion · 2017-12-28T10:04:42Z

tests/python3_f_strings/jsfmt.spec.js

@@ -1 +1 @@
-run_spec(__dirname, ["python"], { pythonVersion: "3" });
+run_spec(__dirname, ["python"], ">=3.6");


might as well... python 3.5 still sees some use, since it's what some of the active ubuntu LTS versions still ship

Elasticbeanstalk (AWS) is stuck to python 3.4, I'd keep support for 3.4+ if possible

taion · 2017-12-28T10:05:34Z

tests_config/run_spec.js

      const source = read(path).replace(/\r\n/g, "\n");

      const mergedOptions = Object.assign({}, options, {
        parser: parsers[0]
      });
      const output = prettyprint(source, path, mergedOptions);
      test(`${filename} - ${mergedOptions.parser}-verify`, () => {
-        expect(raw(source + "~".repeat(80) + "\n" + output)).toMatchSnapshot(
+        expect(raw(source + "~".repeat(79) + "\n" + output)).toMatchSnapshot(


this was a silly change, but 79 makes more sense than 80 if you know it's going to be python code formatted to 79 chars

Should do .repeat(mergedOptions.printWidth). FYI I'd like to eventually share this file from Prettier as a package.

Fixed.

Merging these would be cool. We'd still have to wrap it for Python to do the version thing, though.

taion · 2017-12-28T10:06:14Z

tests/python_with/__snapshots__/jsfmt.spec.js.snap

 with open("tmp/index.html") as f:
    index = f.read()

 with open("tmp/index.html"):
    print("great")

-with A() as X:


and as a result of the build changes, we now automatically flag unnecessary 2/3 inconsistencies like these

(this was the only one)

taion · 2017-12-28T10:06:43Z

src/parser.js

+function parse(text) {
+  const executionResult = spawnSync("python", ["-m", "prettier.parser"], {
+    env: {
+      PATH: process.env.PATH,


this is so the virtualenv python gets picked up. i don't think we need anything else from env here

taion · 2017-12-28T10:19:34Z

.travis.yml

 install:
  - yarn install
-before_install:
-  - export PYTHON_BUILD_CACHE_PATH="/home/travis/.pyenv_cache"
-  - curl -L https://raw.githubusercontent.com/pyenv/pyenv-installer/master/bin/pyenv-installer | bash


Turns out pyenv is already available on Travis (even when the language is node_js), and they have a decent selection of versions already installed.

taion · 2017-12-28T10:20:19Z

.travis.yml

@@ -4,22 +4,19 @@ node_js:
  - 6
  - 8
  - 9
+env:
+  - PYTHON_VERSION=2.7.14


2.7.14 is the current release here; 2.7.11 is two years old and hasn't been current for a year and a half

taion · 2017-12-28T10:21:32Z

python/patched/astexport.py

+            'start': token.startpos,
+            'end': token.endpos,
+        }
+        for token in atok.tokens if token.type == tokenize.COMMENT


resolves https://github.com/prettier/prettier-python/pull/9/files#r158746577 and correspondingly fixes comment handling in python 2

taion · 2017-12-31T00:17:23Z

Any updates here? This PR is unfortunately a mix of a few different things, but I think it sets a lot of pieces in place for further work:

Check that we get the same results for Python 2 and Python 3
Use build matrix for Python versions as well as Node versions
Split out local Python code from vendored/patched Python libraries
Fix a bug with printing "with" statements

I'd like to use this as a starting point for leveraging existing Python tools to make it easier to handle edge cases here.

azz · 2017-12-31T01:28:37Z

tests_config/run_spec.js

@@ -29,16 +39,26 @@ function run_spec(dirname, parsers, options) {
      filename[0] !== "." &&
      filename !== "jsfmt.spec.js"
    ) {
+      if (!semver.satisfies(PYTHON_VERSION, versionRange)) {
+        // Skip each test here with the snapshot name below to avoid obsolete
+        // snapshot failures.


But they are obsolete, right?

I tried to clarify the comment a bit.

The idea is that we run the same tests for both Python 2 and Python 3. However, some of these tests can't be run for Python 2.

For those tests, we skip them when the system Python is Python 2 – but if we don't explicitly skip them like this, then they throw off "obsolete snapshot" failures.

Those failures would be correct if we were only dealing with Python 2, but it's more that they're not pertinent to the test suite as currently run.

taion · 2018-01-04T07:32:48Z

Any updates here? I can rebase this if we think this is the right approach.

patrick91 · 2018-02-13T23:15:56Z

I'll try to have a good look this weekend, thanks, and sorry for the delay :)

patrick91

@taion this is good to go, can you rebase an fix the conflicts? Then I'll merge!

taion · 2018-02-20T23:32:35Z

I don't think I'm going to have time to pick up on this again any time soon. Can someone else take over this PR?

taion · 2018-08-16T18:42:14Z

Not planning on doing further work on this. We've switched to Black for the time being anyway.

taion added 3 commits December 26, 2017 16:06

Update Python integration

2bfe9d7

Better JSON dumping

8224c07

Fix tests

b7b8cf1

taion changed the title ~~[WIP] Update Python integration~~ Update Python integration Dec 27, 2017

Improve tests

33c98cb

taion commented Dec 27, 2017

View reviewed changes

taion added 2 commits December 27, 2017 14:16

Fix with printing

2e096ff

oops

ee7d610

taion commented Dec 27, 2017

View reviewed changes

taion added 3 commits December 27, 2017 14:27

Fix caching

7291885

Fix obsolete snapshot errors

9691a9c

Fix code

ddb74b8

taion commented Dec 28, 2017

View reviewed changes

taion added 2 commits December 28, 2017 02:14

Tweak build

7a1fdd4

Install first just in case

b1f31a0

taion commented Dec 28, 2017

View reviewed changes

Use printWidth

3c47843

Merge branch 'master' into py

a580f63

azz reviewed Dec 31, 2017

View reviewed changes

Clarify comment

4a0c38d

taion mentioned this pull request Jan 14, 2018

Non-injective token to AST mappings #22

Open

taion mentioned this pull request Feb 13, 2018

Upgrade Prettier version, fix comment discovery #25

Merged

patrick91 suggested changes Feb 17, 2018

View reviewed changes

taion closed this Aug 16, 2018


		base_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

		sys.path.insert(0, os.path.join(base_dir, 'vendor'))

		@@ -1 +1 @@
		run_spec(__dirname, ["python"], { pythonVersion: "3" });
		run_spec(__dirname, ["python"], ">=3.6");

Uh oh!

Update Python integration #15

Update Python integration #15

Uh oh!

Conversation

taion commented Dec 27, 2017

Uh oh!

taion commented Dec 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion commented Dec 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion commented Dec 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion Dec 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taion commented Jan 4, 2018

Uh oh!

patrick91 commented Feb 13, 2018

Uh oh!

patrick91 left a comment

Choose a reason for hiding this comment

Uh oh!

taion commented Feb 20, 2018

Uh oh!

taion commented Aug 16, 2018

Uh oh!

Uh oh!

taion Dec 31, 2017 •

edited

Loading